Skip to content

Add callback to fetch latest release for a repo#2

Open
acrollet wants to merge 4 commits into
skwashd:7.x-1.xfrom
Roomify:7.x-1.x
Open

Add callback to fetch latest release for a repo#2
acrollet wants to merge 4 commits into
skwashd:7.x-1.xfrom
Roomify:7.x-1.x

Conversation

@acrollet

@acrollet acrollet commented Feb 2, 2016

Copy link
Copy Markdown

Adds callbacks to:

  • fetch latest github release
  • fetch a binary release asset file by ID and store as a managed file

Comment thread github_api.module

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acrollet It's great to see you adding more functionality to the library.

The idea of this module is that it is a lightweight wrapper around [https://github.com/KnpLabs/php-github-api](KnP Lab's github library). They have implemented support for release assets. The composer.json should be updated to so the dependency is ^1.5.1 and then instead of using drupal_http_request() to make this call it should use the library.

I haven't run coder-review over the patch, but other than the issue above I don't see any other issues with the patch. If you fix it and squash the commits it should be an easy merge.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skwashd their support does not actually fetch a binary asset (from a private repo) as this code does, the get() method simply returns an object describing a github asset. I looked into adding a fetch method to the library, but it would be a non-trivial addition. c.f. KnpLabs/php-github-api#320

Fix release check

Add callback to fetch release asset by ID and store as a managed file.

Document how to avoid storing GH password

Revert changes to README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants